fix(@angular/build): prevent concurrent stylesheet bundling esbuild context leaks#33318
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces request deduplication and write tracking to the Cache class to handle concurrent getOrCreate calls safely, alongside improvements to BundlerContext to manage active bundle promises and disposal. The review feedback highlights a potential race condition in Cache.getOrCreate where an active request might be incorrectly deleted after an await gap, a failing assertion in the new unit tests regarding promise resolution after a cache override, and a potential memory leak due to #writeCounts entries never being cleaned up.
983db14 to
01404ec
Compare
…ontext leaks When multiple components in the same file define identical inline styles, concurrent calls to the stylesheet bundler invoke Cache.getOrCreate concurrently with the same key. Because getOrCreate is not concurrency-safe, multiple BundlerContext instances are created. Additionally, if multiple bundle() requests occur concurrently on a BundlerContext when the esbuild context is not yet initialized, they both invoke context(), causing one to overwrite and leak the other. Leaked esbuild contexts keep the Node event loop active forever, hanging the test runner. Resolve the Cache race by using an internal Map of in-flight promises to share the same active request, along with a version counter to detect mid-flight writes and safely retry. Resolve the BundlerContext race by memoizing and sharing the active performBundle promise. Fixes angular#33317
01404ec to
0c80537
Compare
|
This pull request has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
When multiple components in the same file define identical inline styles, concurrent calls to the stylesheet bundler invoke Cache.getOrCreate concurrently with the same key. Because getOrCreate is not concurrency-safe, multiple BundlerContext instances are created.
Additionally, if multiple bundle() requests occur concurrently on a BundlerContext when the esbuild context is not yet initialized, they both invoke context(), causing one to overwrite and leak the other. Leaked esbuild contexts keep the Node event loop active forever, hanging the test runner.
Resolve the Cache race by using an internal Map of in-flight promises to share the same active request, along with a version counter to detect mid-flight writes and safely retry. Resolve the BundlerContext race by memoizing and sharing the active performBundle promise.
Fixes #33317